Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rescue exceptions raised when calling to_s while sanitizing #361

Merged
merged 2 commits into from
Jun 15, 2017

Conversation

kattrali
Copy link
Contributor

In the case that calling to_s will raise, like in ActionDispatch::RemoteIp::IpSpoofAttackError, rescue and insert a placeholder.

Relevant example

@kattrali kattrali requested a review from snmaynard June 13, 2017 22:46
@snmaynard
Copy link
Contributor

Does this mean that every to_s will display [SPOOF] if it throws? Any reason we cant to_s earlier the ip in the middleware earlier and rescue [SPOOF] there?

When calling to_s will raise, rescue and insert a placeholder.
If a ActionDispatch::RemoteIp instance cannot be memoized, it throws an
IpSpoofAttackError from to_s. In this case, replace the IP with a
placeholder.

Ref: 
https://github.com/rails/rails/blob/34fe2a4fc778d18b7fe6bdf3629c1481bee789b9/actionpack/lib/action_dispatch/middleware/remote_ip.rb#L134
@kattrali
Copy link
Contributor Author

We can.

@kattrali kattrali force-pushed the kattrali/avoid-raising-in-client_ip branch from 758a030 to 8e4389d Compare June 14, 2017 00:46
@kattrali kattrali merged commit 2ee97f5 into master Jun 15, 2017
@kattrali kattrali deleted the kattrali/avoid-raising-in-client_ip branch June 15, 2017 18:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants